Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/promote/managedscripts/managed_scripts.go`:
- Around line 30-32: The FilterTargets method currently hardcodes
prodNamespaceRef when calling utils.FilterTargetsContainingNamespaceRef, so the
configured --namespaceRef value is ignored; update
promoteCallbacks.FilterTargets to pass the instance/configured namespace
reference (e.g., c.namespaceRef or the field that holds the CLI flag) into
utils.FilterTargetsContainingNamespaceRef instead of prodNamespaceRef, ensuring
the same change is applied where else prodNamespaceRef is used for target
selection so the runtime flag actually influences filtering.

In `@cmd/promote/saas/saas.go`:
- Line 234: Update the CLI help/example string to use the canonical flag
--serviceId instead of the legacy alias --serviceName: locate the usage text
that currently reads `osdctl promote saas --serviceName <service> --gitHash
<git-hash>` (in the promote command's help/usage output) and replace
`--serviceName` with `--serviceId`; ensure any adjacent help text or examples
referencing the legacy alias are also updated so the example consistently shows
`--serviceId <service>` with `--gitHash <git-hash>`.
- Around line 271-274: Add a fail-fast required check for ops.serviceId before
calling servicesRegistry.GetService to give a clearer error for promote mode:
validate that ops.serviceId is non-empty (e.g., at the start of the promote
handler or before the GetService call), and return a descriptive error if it's
empty (mentioning that --serviceId is required in promote mode). Update the code
path where servicesRegistry.GetService(ops.serviceId) is invoked (reference
ops.serviceId and the promote handler function) to perform this explicit check
and only call GetService when the value is present.
- Around line 38-43: The current logic returns the original filePath when
deploy.yaml is missing, which can register a directory as a service; change the
code to only return valid regular file paths: after checking subFilePath, also
verify that filePath itself is a regular file (use os.Stat(filePath) and
fileInfo.Mode().IsRegular()) before returning it, and if it's a directory or not
a regular file return an empty string (or an error sentinel) so the caller can
skip this SaaS entry; adjust callers of this helper to treat empty string as
"skip invalid entry".

In `@cmd/promote/utils/git_repo.go`:
- Around line 54-89: The code pre-marks commonAncestorCommit.Hash in
visitedHashes before traversal which allows walks that never actually reach
commonAncestorHash to succeed silently; modify the traversal in the function
containing queue, visitedHashes, commit.IsAncestor and the final sb.WriteString
branch so that you either do not pre-populate visitedHashes with
commonAncestorCommit.Hash or (preferably) add a post-traversal check that
verifies visitedHashes contains commonAncestorHash (or that you visited it
during traversal) and return an explicit error if it was not reached; use the
symbols visitedHashes, commonAncestorCommit / commonAncestorHash, queue,
commit.IsAncestor and the final return path to locate where to add the check.

In `@cmd/promote/utils/service.go`:
- Around line 212-229: The function FilterTargetsContainingNamespaceRef
currently uses strings.Contains(visitedNamespaceRef, namespaceRef) which allows
substring matches; change the check to a precise match (e.g., verify the path
ends with the expected segment or exact identifier) by replacing the contains
logic with a more strict comparison such as using strings.HasSuffix or parsing
visitedNamespaceRef into path segments and comparing the final segment to
namespaceRef (operate on visitedNamespaceRef and namespaceRef variables inside
FilterTargetsContainingNamespaceRef so only exact/segment matches are
considered).
- Around line 141-146: The code reads appRelFilePath from
yamlDoc.rootNode.GetString and directly uses
filepath.Join(appInterfaceClone.GetPath(), "data", appRelFilePath) before
calling readApplicationFromFile; sanitize appRelFilePath by running
filepath.Clean, reject absolute paths or any path that resolves outside the
intended base (e.g. contains ".." segments that escape the
appInterfaceClone.GetPath()/data directory), and construct the final path by
joining the cleaned relative path to appInterfaceClone.GetPath()/data then
verifying the resulting path has the base directory as a prefix before calling
readApplicationFromFile.
- Around line 389-394: Iterate deterministically over oldHashToTargetNodes by
collecting its keys into a slice, sorting that slice (using sort.Strings), and
then building resourceTemplatePromotions in the sorted order (instead of ranging
the map directly); update the import block to include "sort". Ensure the same
symbols are used: oldHashToTargetNodes, resourceTemplatePromotions,
resourceTemplatePromotion, and that downstream code that calls promote will now
see a stable commit order.

In `@cmd/promote/utils/services_registry.go`:
- Around line 19-33: The constructor NewServicesRegistry should guard against
nil inputs to avoid panics: at the start of the function check if
appInterfaceClone is nil and if so return a descriptive error (do not call
appInterfaceClone.GetPath()); also check if validateServiceFilePathCallback is
nil and return a descriptive error before invoking it later; update
callers/tests if necessary to pass non-nil values or to handle the returned
error from NewServicesRegistry.
- Around line 36-37: The code silently overwrites existing entries in the
serviceIdToFilePath map (variable serviceId) when scanning files; change the
insertion logic to fail fast on duplicates by checking if serviceId already
exists in serviceIdToFilePath and returning/propagating an error (or logging and
exiting) that includes the conflicting serviceId and both file paths (existing
value and serviceFilePath) so callers of the scanning function can handle the
failure instead of allowing silent overwrite.

In `@docs/osdctl_promote_managedscripts.md`:
- Around line 5-39: Add language identifiers to all fenced code blocks in
docs/osdctl_promote_managedscripts.md to satisfy MD040: mark the command usage
block as "text" (or "bash" if preferred), the example block as "bash", and both
options blocks (Options and Options inherited from parent commands) as "text".
Update the fences surrounding the snippets that contain "osdctl promote
managedscripts [flags]", the example starting with "# Promote managed-scripts
repo", the options list beginning with "--appInterfaceDir", and the inherited
options list beginning with "--as" to include the appropriate language tags.

In `@docs/osdctl_promote_saas.md`:
- Around line 22-30: The fenced code block showing CLI flags in the saas
promotion docs lacks a language identifier which triggers markdownlint MD040;
update the triple-backtick opening fence to include a language (e.g., ```text)
for the block containing the lines starting with "--appInterfaceDir" and ending
with "--serviceId" so the block is treated as plain text and the linter warning
is resolved.
- Line 17: The example for the CLI invocation uses the deprecated alias flag
--serviceName; update the example for the command osdctl promote saas to use the
canonical flag --serviceId instead (replace the --serviceName token with
--serviceId in the example line) so the documentation shows the current,
supported flag.

In `@docs/README.md`:
- Around line 3938-3940: Add a language identifier to the fenced code block that
contains the line "osdctl promote managedscripts [flags]" so markdownlint MD040
is satisfied; modify the opening fence from ``` to ```text (or ```bash) around
the block in README.md so it becomes ```text followed by the command and closing
``` to preserve formatting.

---

Outside diff comments:
In `@cmd/promote/dynatrace/dt_utils.go`:
- Around line 175-178: The Open(filename) call currently prints the error and
continues, which can pass an invalid file handle into downstream code; update
the error handling at the Open(...) site so that on err you return the error (or
otherwise abort the function) instead of just fmt.Println(err). Locate the
Open(filename) invocation in dt_utils.go and modify the surrounding function to
propagate the error (or call return) immediately when err != nil, ensuring
downstream code does not receive a nil/invalid file handle.
- Around line 142-153: The os.ReadDir calls in the nested directory walk (the
reads that populate items, subitems and subitems2) currently ignore their
returned errors; change each call to capture the error (e.g., items, err :=
os.ReadDir(dir)) and propagate it instead of discarding it—return the error (or
wrap and return) from the enclosing function (the directory-walking function in
dt_utils.go) so failures stop the promotion; do the same for subitems and
subitems2 reads and adjust calling code to handle the returned error.

In `@cmd/promote/dynatrace/dynatrace.go`:
- Around line 73-74: The calls to listDynatraceModuleNames currently discard the
returned error and then call os.Exit(0); change both call sites to capture the
error (e.g., err := listDynatraceModuleNames(dynatraceConfig)), check if err !=
nil, print or log the error to stderr (or use the existing logger) and call
os.Exit(1); if no error, then exit with success. This ensures failures from
listDynatraceModuleNames are surfaced instead of being silently ignored.

---

Nitpick comments:
In `@cmd/promote/dynatrace/dt_utils.go`:
- Around line 108-124: GetModulesNames currently appends to package globals
ModulesSlice and ModulesFilesMap on each call; modify GetModulesNames to
reset/clear ModulesSlice and reinitialize or clear ModulesFilesMap at the start
of the function (before reading dir entries) so repeated calls don't accumulate
stale data; locate the function GetModulesNames and add logic to set
ModulesSlice = nil (or empty slice) and ModulesFilesMap =
make(map[string]string) (or clear existing entries) before populating them.

In `@cmd/promote/utils/service.go`:
- Around line 42-44: The GetName method for type yamlDoc uses an inconsistent
receiver name `s`; change its receiver to `d` to match the other methods on
yamlDoc (e.g., GetFilePath and Save) so all methods use the same receiver
identifier; update the method signature for GetName to use `d` and keep the body
returning d.name.
- Around line 413-419: Remove the extra blank line in the error handling after
calling repo.GetHeadHash so the idiomatic Go pattern is preserved; specifically,
in the block that assigns newHash (the call to repo.GetHeadHash) and the
subsequent if err != nil block, bring the if directly under the call (no blank
line) to keep the standard newHash, err := repo.GetHeadHash / if err != nil
style used elsewhere.
- Around line 296-330: The promote function (resourceTemplatePromotion.promote)
commits each resource independently (uses callbacks.SetTargetHash, service.Save,
service.appInterfaceClone.Commit) which can leave the local branch partially
committed if a later promotion fails; update the caller that loops over
promotions (the code that invokes resourceTemplatePromotion.promote) to track
which promotions succeeded and, on any subsequent error, emit a clear
warning/log that lists the committed resource relPaths and informs the user that
the local branch contains partial commits requiring manual push/revert;
alternatively expose a boolean or error type from promote indicating "committed"
so the caller can build that list before logging the warning.
- Line 21: The exported function ReadYamlDocFromFile returns an unexported type
yamlDoc which prevents external callers from using its value; rename the type to
exported YamlDoc (or change the function to return an exported interface) and
update all references accordingly (including ReadYamlDocFromFile's return type
and any uses in cmd/promote/saas/saas.go). Also standardize method receiver
names on that type: change the GetName receiver from 's' to 'd' to match
GetFilePath and Save so all methods on yamlDoc/YamlDoc use a consistent receiver
(d). Ensure you update any imports/usages and tests to the new exported type
name.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/promote/dynatrace/dynatrace.go`:
- Around line 93-117: Move the checks that validate non-terraform flags (the
ops.list, ops.component and ops.gitHash logic and the error returns like "--list
cannot be used with --component or --gitHash" and "--component is required
unless --list is used") to run before calling utils.FindAppInterfaceClone and
utils.NewServicesRegistry so missing/invalid flags are caught immediately; i.e.,
validate ops.list/ops.component/ops.gitHash at the top of the function and
return the appropriate errors (or call listServiceIds when ops.list) before
invoking FindAppInterfaceClone or NewServicesRegistry.

In `@cmd/promote/managedscripts/managed_scripts.go`:
- Around line 64-70: The command registered in the Cobra command definition uses
Use: "managedscripts" which does not match the advertised "managed-scripts";
update the Cobra command in managed_scripts.go to either change the Use value to
"managed-scripts" or add an Alias entry including "managedscripts" (or both) so
both forms work; locate the command struct where Use, Short, Args,
DisableAutoGenTag and Example are set and modify the Use/Aliases fields
accordingly to ensure "osdctl promote managed-scripts" is recognized.

In `@cmd/promote/utils/services_registry_test.go`:
- Around line 39-59: The test asserts that GetServicesIds() advertises
"service-4" but GetService("service-4") fails, creating a contract mismatch;
either make GetService succeed for advertised services or stop advertising it.
Fix by updating the test fixture or registry setup used by
services_registry_test.go so the declared service IDs from GetServicesIds()
match retrievable services from GetService() — e.g., ensure the underlying
registry data includes a valid entry/file for "service-4" (so GetService returns
a non-nil service and no error) or remove "service-4" from the expected slice in
the GetServicesIds() assertion; keep references to GetServicesIds and GetService
when applying the change.

---

Outside diff comments:
In `@cmd/promote/dynatrace/dt_utils.go`:
- Around line 137-166: The function updatePromotionGitHash currently ignores
errors returned by os.ReadDir (when listing dir, subDir, and subDir2), which can
hide permission or missing-directory problems; update each os.ReadDir call to
capture its error, check it, and return a wrapped error (including the path and
original error) instead of proceeding silently so callers can fail fast—apply
this to the top-level items, subitems and subitems2 reads in
updatePromotionGitHash and propagate or return errors from updateFileContent as
appropriate.

---

Duplicate comments:
In `@cmd/promote/dynatrace/dynatrace.go`:
- Around line 126-131: The code currently calls fmt.Printf and os.Exit(1) after
service.Promote fails, which prevents Cobra's RunE error handling and deferred
cleanup; change the block in the function that calls
service.Promote(&utils.DefaultPromoteCallbacks{Service: service}, ops.gitHash)
to return the error (or a wrapped error with context) instead of printing and
exiting so the caller/RunE can handle it and deferred cleanup runs; specifically
remove fmt.Printf/os.Exit and return err (or fmt.Errorf("promote failed: %w",
err)) from the surrounding command handler.

---

Nitpick comments:
In `@cmd/promote/dynatrace/dt_utils.go`:
- Around line 25-28: ModulesSlice and ModulesFilesMap are package-level mutable
variables that GetModulesNames appends to, causing test pollution; change
GetModulesNames to use and return fresh local variables (e.g., local
modulesSlice []string and modulesFilesMap map[string]string) instead of
appending to the package-level ModulesSlice/ModulesFilesMap, and update callers
to accept the returned slice/map, or alternatively clear ModulesSlice and
ModulesFilesMap at the start of GetModulesNames before use; apply the same
local/clear fix to the other similar code block referenced around lines 106-123
(same function or helper) so no package-level mutation persists across
calls/tests.

In `@cmd/promote/utils/app_interface_clone.go`:
- Around line 110-128: CheckoutNewBranch currently hardcodes "master"; change it
to detect the repository's current/default branch instead and use that for the
initial checkout. In CheckoutNewBranch use repo.Head() (or resolve
"refs/remotes/origin/HEAD" if you need remote default) to obtain the default
branch reference name and pass that into workTree.Checkout instead of
plumbing.NewBranchReferenceName("master"), falling back to "master" or "main"
only if detection fails; keep existing logic for branchReference creation,
repo.Reference check, RemoveReference and final Checkout(Create:true).

In `@cmd/promote/utils/git_repo.go`:
- Around line 70-75: The warning string in Repo.Cleanup uses "Warning:Failed"
without a space; update the fmt.Printf call inside Cleanup (function
Repo.Cleanup) to insert a space after the colon (i.e., "Warning: Failed to
remove clone directory '%s': %v") so the message matches the formatting used
elsewhere (referencing r.clonePath and the fmt.Printf call).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/promote/utils/app_interface_clone.go`:
- Around line 77-94: The fallback branch currently prints a diagnostic to stdout
during normal lookup attempts (the block that calls os.Getwd,
git.PlainOpenWithOptions and newAppInterfaceClone around AppInterfaceClone)
which pollutes output; remove or silence that fmt.Printf so the function does
not write to stdout during successful fallback attempts and only emits
diagnostics on final failure or via verbose/stderr logging. Change the final
diagnostic to return the error (or print to os.Stderr using
fmt.Fprintln(os.Stderr, ...)) only when no repo was found, and ensure
intermediate errors from os.Getwd, git.PlainOpenWithOptions, and
newAppInterfaceClone are not printed to stdout during normal control flow.
- Around line 110-137: Before mutating the repo, ensure the work tree is clean:
call a.workTree.Status() and if !status.IsClean() return an error refusing to
proceed; add this check at the start of CheckoutNewBranch (and optionally in
Commit) so CheckoutNewBranch and Commit do not stage unrelated edits via
a.workTree.AddGlob("."). Keep existing logic for deleting and creating branches
(repo.Reference, repo.Storer.RemoveReference, workTree.Checkout) intact but gate
them behind the cleanliness check and return a clear error mentioning the dirty
working tree and a.path.

In `@cmd/promote/utils/service.go`:
- Around line 301-324: The promote flow (function
resourceTemplatePromotion.promote) currently calls repo.ResolveHash and then
writes service.Save and SetTargetHash, which lets an invalid newHash cause files
to be rewritten before git commit/branch operations; validate that newHash
resolves to an existing ref before mutating the working copy by calling
repo.ResolveHash (or an explicit repo.ValidateRef) and returning an error if it
doesn't resolve, and only after successful validation proceed to invoke
callbacks.SetTargetHash, service.Save, callbacks.ComputeCommitMessage and
service.appInterfaceClone.Commit; apply the same pre-validation change to the
other similar promotion path that uses
callbacks.SetTargetHash/ComputeCommitMessage and service.Save (the block
referenced around the second occurrence of the promote flow).

---

Duplicate comments:
In `@cmd/promote/dynatrace/dynatrace.go`:
- Around line 93-117: The CLI flag validation for non-terraform operations
should run before cloning the repo and constructing the services registry: move
the checks that validate ops.list, ops.component, and ops.gitHash (the logic
that returns errors for "--list cannot be used with --component or --gitHash"
and "--component is required unless --list is used") above the calls to
utils.FindAppInterfaceClone and utils.NewServicesRegistry; if ops.list is true
preserve setting cmd.SilenceUsage = true and directly call
listServiceIds(servicesRegistry) only after constructing the registry when
actually needed, but prevent any repo clone/registry setup when basic flag
validation would already return an error. Ensure the condition ordering around
ops.list, ops.component and ops.gitHash is performed first so invalid flag
combinations short-circuit before invoking FindAppInterfaceClone or
NewServicesRegistry.
- Around line 128-130: The code inside the Cobra command's RunE handler (the
RunE closure in dynatrace.go that currently prints the error and calls
os.Exit(1)) should not exit the process; instead remove the fmt.Printf and
os.Exit(1) and return the error to let Cobra handle it. Update the error branch
in the RunE closure (the block that checks if err != nil after promoting the
service) to return a wrapped or raw err (e.g., fmt.Errorf("promote service: %w",
err) or simply return err) so tests and Cobra's error handling can observe the
failure.

In `@cmd/promote/saas/saas.go`:
- Around line 34-45: validateSaasServiceFilePath currently can return a
directory path when a "saas-*" entry is a directory and deploy.yaml is missing;
change the logic so directories are never returned: in
validateSaasServiceFilePath, after verifying
strings.HasPrefix(filepath.Base(filePath), "saas-"), stat filePath and if it's a
directory then only return filepath.Join(filePath, "deploy.yaml") when that file
exists and is regular (as checked now); if deploy.yaml is missing return "" (do
not return the directory); if filePath itself is a regular file return filePath
as before.

In `@cmd/promote/utils/services_registry.go`:
- Around line 35-37: Change the code that assigns serviceIdToFilePath[serviceId]
= serviceFilePath to detect duplicates and reject them instead of overwriting:
before inserting, check if serviceId already exists in serviceIdToFilePath; if
it does, return an error (or fail construction) that includes the conflicting
serviceId and both file paths (existing and new) so the caller can detect which
scanned directories produced the collision. Update the constructor/loader
function that iterates fileName/serviceFilePath to perform this check and
propagate the error back to callers.

---

Nitpick comments:
In `@cmd/promote/dynatrace/dt_utils.go`:
- Around line 25-28: The globals ModulesSlice and ModulesFilesMap make module
discovery stateful; change GetModulesNames to allocate and return fresh []string
and map[string]string local variables (do not append to package-scope
ModulesSlice/ModulesFilesMap), update callers (e.g., ValidateModuleName) to
accept or call GetModulesNames for fresh data instead of reading globals, and
remove or replace the append-only globals (and any similar package-scope caches
around lines 106-123) so each call computes a new module list/map.